-
Notifications
You must be signed in to change notification settings - Fork 305
Create Party enum and deprecate ElligatorSwiftParty in favor of it
#752
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
8337acc to
c83c5a7
Compare
src/ellswift.rs
Outdated
| } | ||
|
|
||
| #[allow(deprecated)] | ||
| #[allow(dead_code)] // We aren't using this anymore in this library, but users might be using it |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In c83c5a7:
How can users be using this if it's a private method?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure what i was thinking - removed this bit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not resolved. Now this is dead code and is causing a local CI failure for me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In 3b7d393
|
Also, we should consider backporting this to the version used by rust-bitcoin 0.32. |
c83c5a7 to
826f884
Compare
|
This PR is doing two things, one of which is not in the title so is obfuscated. Please either do two separate PRs or describe the whole change set (probably still requires two patches). |
86fe99a to
e04237b
Compare
Party enum and deprecate ElligatorSwiftParty in favor of it
d9d378e to
e30f8f8
Compare
Introducing this enum as a replacement for ElligatorSwiftParty. Party is more descriptive and should cause less confusion than ElligatorSwiftParty
e30f8f8 to
bf2c35c
Compare
|
@tcharding done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK bf2c35c
|
The deprecation stuff is particularly nice, well thought out. |
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 3b7d393; successfully ran local tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 3b7d393; successfully ran local tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK bf2c35c; successfully ran local tests
|
Lol, there we go, I got it. I had been sorting commits alphabetically instead of topologically when deciding what the "tip" was. |
f12bde6 Deprecate `ElligatorSwiftParty` in favor of `Party` (Shing Him Ng) 33fda15 Create `Party` enum (Shing Him Ng) Pull request description: Also, we should consider backporting this to the version used by rust-bitcoin 0.32. _Originally posted by apoelstra in #752 (comment) Backport #752 to the [version used by rust-bitcoin 0.32](https://github.com/rust-bitcoin/rust-bitcoin/blob/7af9e33f2b9033cf2701725eba280e14ebda0cf5/bitcoin/Cargo.toml#L35) ACKs for top commit: apoelstra: ACK f12bde6; successfully ran local tests Tree-SHA512: e8184c0df1f19a6512b1168bb1cf49e906de6d7f51ef1f9a4e3977422c36e603c3325fedb1485efa49ea8cb0361b54a293cdfefef10f3370541c8086b2b28bff
…ate `ElligatorSwiftParty` in favor of it
bf2c35c1a8d391aa1b5a06e94c5c9a14dc69a7fb Deprecate `ElligatorSwiftParty` in favor of `Party` (Shing Him Ng)
3b7d3938873eb62406dd4d52094deeabddb0074f Create `Party` enum (Shing Him Ng)
Pull request description:
The initial naming of ElligatorSwiftParty wasn't very descriptive, so it will be deprecated in favor of a more descriptive `Party` enum. I updated `shared_secret` and `shared_secret_with_hasher` to accept the new `Party` enum as well - I'm not sure if there's a better way to do it, but changing it to an `impl Into<Party>` should preserve backwards compatibility
Fixes #741
ACKs for top commit:
tcharding:
ACK bf2c35c1a8d391aa1b5a06e94c5c9a14dc69a7fb
apoelstra:
ACK bf2c35c1a8d391aa1b5a06e94c5c9a14dc69a7fb; successfully ran local tests
Tree-SHA512: c516b8797b53e8e4167666ee6c93be61f67f2e71d33ba7354d6432199bd1f80680eea030c0c00ee5c0ba23204439d8c63c0efb8fc753f13e4cec189f7eee9a36
…ate `ElligatorSwiftParty` in favor of it
bf2c35c1a8d391aa1b5a06e94c5c9a14dc69a7fb Deprecate `ElligatorSwiftParty` in favor of `Party` (Shing Him Ng)
3b7d3938873eb62406dd4d52094deeabddb0074f Create `Party` enum (Shing Him Ng)
Pull request description:
The initial naming of ElligatorSwiftParty wasn't very descriptive, so it will be deprecated in favor of a more descriptive `Party` enum. I updated `shared_secret` and `shared_secret_with_hasher` to accept the new `Party` enum as well - I'm not sure if there's a better way to do it, but changing it to an `impl Into<Party>` should preserve backwards compatibility
Fixes #741
ACKs for top commit:
tcharding:
ACK bf2c35c1a8d391aa1b5a06e94c5c9a14dc69a7fb
apoelstra:
ACK bf2c35c1a8d391aa1b5a06e94c5c9a14dc69a7fb; successfully ran local tests
Tree-SHA512: c516b8797b53e8e4167666ee6c93be61f67f2e71d33ba7354d6432199bd1f80680eea030c0c00ee5c0ba23204439d8c63c0efb8fc753f13e4cec189f7eee9a36
The initial naming of ElligatorSwiftParty wasn't very descriptive, so it will be deprecated in favor of a more descriptive
Partyenum. I updatedshared_secretandshared_secret_with_hasherto accept the newPartyenum as well - I'm not sure if there's a better way to do it, but changing it to animpl Into<Party>should preserve backwards compatibilityFixes #741